Add a distinct `OperandValue::ZeroSized` variant for ZSTs #111318

scottmcm · 2023-05-07T10:19:28Z

These tend to have special handling in a bunch of places anyway, so the variant helps remember that. And I think it's easier to grok than Aggregates sometimes being Immediates (after all, I previously got that wrong and caused #109992). As a minor bonus, it means we don't need to generate poison LLVM values for ZSTs to pass around in OperandValue::Immediates.

Inspired by #110021 (comment), so
r? @compiler-errors

rustbot · 2023-05-07T10:19:35Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

compiler/rustc_middle/src/mir/mod.rs

Noratrieb · 2023-05-22T08:40:01Z

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -220,31 +223,33 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                let fake_place = PlaceRef::new_sized_aligned(cast_ptr, cast, align);
                Some(bx.load_operand(fake_place).val)
            }
+            OperandValue::ZeroSized => {
+                let OperandValueKind::ZeroSized = operand_kind else {


Could this be an assert instead?

It could. I wrote it this way for structural similarity with the other arms.

Maybe I should have made this be match (operand.val, operand_kind) instead? Unsure.

Itś not that big of a deal, both branches are pretty short

compiler-errors · 2023-05-29T04:57:38Z

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -220,31 +223,33 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                let fake_place = PlaceRef::new_sized_aligned(cast_ptr, cast, align);
                Some(bx.load_operand(fake_place).val)
            }
+            OperandValue::ZeroSized => {
+                let OperandValueKind::ZeroSized = operand_kind else {


Itś not that big of a deal, both branches are pretty short

compiler-errors · 2023-05-29T04:58:38Z

@bors r+

bors · 2023-05-29T04:58:40Z

📌 Commit f315f57db8721e0993d9866c2cd5c3cae0fb6700 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-05-29T06:12:12Z

⌛ Testing commit f315f57db8721e0993d9866c2cd5c3cae0fb6700 with merge 28d692dccbadccb687ae571daf3591510a451529...

bors · 2023-05-29T07:10:37Z

💔 Test failed - checks-actions

scottmcm · 2023-05-29T07:12:32Z

Looks like a legit failure; I'll dig in.

@bors r-

scottmcm · 2023-05-29T15:31:33Z

Ah, so we do need to pass ZSTs in some ABIs.

Fix: https://github.com/rust-lang/rust/compare/b95bfa5b2db64d5e985e8a6fa83358df3df345aa..9495095fe68b737254a20e39b3c744ca41e66d6f
Confirmation that makes the job pass: https://github.com/rust-lang/rust/actions/runs/5109977534/jobs/9185355643?pr=111318

@bors r=compiler-errors

bors · 2023-05-29T15:31:35Z

📌 Commit 9495095 has been approved by compiler-errors

It is now in the queue for this repository.

…ompiler-errors Add a distinct `OperandValue::ZeroSized` variant for ZSTs These tend to have special handling in a bunch of places anyway, so the variant helps remember that. And I think it's easier to grok than `Aggregate`s sometimes being `Immediates` (after all, I previously got that wrong and caused rust-lang#109992). As a minor bonus, it means we don't need to generate poison LLVM values for ZSTs to pass around in `OperandValue::Immediate`s. Inspired by rust-lang#110021 (comment), so r? `@compiler-errors`

…ompiler-errors Add a distinct `OperandValue::ZeroSized` variant for ZSTs These tend to have special handling in a bunch of places anyway, so the variant helps remember that. And I think it's easier to grok than `Aggregate`s sometimes being `Immediates` (after all, I previously got that wrong and caused rust-lang#109992). As a minor bonus, it means we don't need to generate poison LLVM values for ZSTs to pass around in `OperandValue::Immediate`s. Inspired by rust-lang#110021 (comment), so r? ``@compiler-errors``

…ompiler-errors Add a distinct `OperandValue::ZeroSized` variant for ZSTs These tend to have special handling in a bunch of places anyway, so the variant helps remember that. And I think it's easier to grok than `Aggregate`s sometimes being `Immediates` (after all, I previously got that wrong and caused rust-lang#109992). As a minor bonus, it means we don't need to generate poison LLVM values for ZSTs to pass around in `OperandValue::Immediate`s. Inspired by rust-lang#110021 (comment), so r? ```@compiler-errors```

matthiaskrgr · 2023-05-31T09:11:14Z

@bors r- #112130

These tend to have special handling in a bunch of places anyway, so the variant helps remember that. And I think it's easier to grok than non-Scalar Aggregates sometimes being `Immediates` (like I got wrong and caused 109992). As a minor bonus, it means we don't need to generate poison LLVM values for them to pass around in `OperandValue::Immediate`s.

scottmcm · 2023-06-01T02:51:35Z

Rebased to resolve merge with #111768
@bors r=compiler-errors

bors · 2023-06-01T02:51:37Z

📌 Commit bf36193 has been approved by compiler-errors

It is now in the queue for this repository.

Rollup of 7 pull requests Successful merges: - rust-lang#108459 (rustdoc: Fix LinkReplacer link matching) - rust-lang#111318 (Add a distinct `OperandValue::ZeroSized` variant for ZSTs) - rust-lang#111892 (rustdoc: add interaction delays for tooltip popovers) - rust-lang#111980 (Preserve substs in opaques recorded in typeck results) - rust-lang#112024 (Don't suggest break through nested items) - rust-lang#112128 (Don't compute inlining status of mono items in advance.) - rust-lang#112141 (remove reference to Into in ? operator core/std docs, fix rust-lang#111655) Failed merges: - rust-lang#112071 (Group rfcs tests) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned compiler-errors May 7, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 7, 2023

cjgillot reviewed May 7, 2023

View reviewed changes

compiler/rustc_middle/src/mir/mod.rs Outdated Show resolved Hide resolved

scottmcm force-pushed the operand-value-poison branch from ebcd632 to f315f57 Compare May 7, 2023 18:15

Noratrieb reviewed May 22, 2023

View reviewed changes

compiler-errors approved these changes May 29, 2023

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2023

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 29, 2023

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2023

scottmcm force-pushed the operand-value-poison branch 2 times, most recently from b95bfa5 to 9495095 Compare May 29, 2023 08:12

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels May 29, 2023

scottmcm force-pushed the operand-value-poison branch from 4e90953 to 9495095 Compare May 29, 2023 15:30

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 29, 2023

scottmcm removed A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels May 29, 2023

Noratrieb mentioned this pull request May 30, 2023

Rollup of 6 pull requests #112092

Closed

Noratrieb mentioned this pull request May 30, 2023

Rollup of 5 pull requests #112095

Closed

matthiaskrgr mentioned this pull request May 31, 2023

Rollup of 6 pull requests #112130

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 31, 2023

scottmcm closed this May 31, 2023

scottmcm reopened this May 31, 2023

This comment has been minimized.

Sign in to view

scottmcm force-pushed the operand-value-poison branch from 9495095 to bf36193 Compare June 1, 2023 02:11

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 1, 2023

Dylan-DPC mentioned this pull request Jun 1, 2023

Rollup of 7 pull requests #112164

Merged

bors merged commit 03d4299 into rust-lang:master Jun 1, 2023

rustbot added this to the 1.72.0 milestone Jun 1, 2023

scottmcm deleted the operand-value-poison branch June 2, 2023 03:12

Add a distinct OperandValue::ZeroSized variant for ZSTs #111318

Add a distinct OperandValue::ZeroSized variant for ZSTs #111318

Uh oh!

Conversation

scottmcm commented May 7, 2023

Uh oh!

rustbot commented May 7, 2023

Uh oh!

Uh oh!

Noratrieb May 22, 2023

Choose a reason for hiding this comment

Uh oh!

scottmcm May 23, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors May 29, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors May 29, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented May 29, 2023

Uh oh!

bors commented May 29, 2023

Uh oh!

bors commented May 29, 2023

Uh oh!

This comment has been minimized.

bors commented May 29, 2023

Uh oh!

scottmcm commented May 29, 2023

Uh oh!

scottmcm commented May 29, 2023

Uh oh!

bors commented May 29, 2023

Uh oh!

matthiaskrgr commented May 31, 2023

Uh oh!

This comment has been minimized.

scottmcm commented Jun 1, 2023

Uh oh!

bors commented Jun 1, 2023

Uh oh!

Uh oh!

Add a distinct `OperandValue::ZeroSized` variant for ZSTs #111318

Add a distinct `OperandValue::ZeroSized` variant for ZSTs #111318